Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetchPriority enabled by default in Firefox 132 #24518

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

acreskeyMoz
Copy link

@acreskeyMoz acreskeyMoz commented Sep 25, 2024

Summary

We're enabling fetchpriority by default in Fx 132

Test results and supporting details

https://bugzilla.mozilla.org/show_bug.cgi?id=1854077

Related issues

mdn/content#36121

@github-actions github-actions bot added data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API data:html 📄 Compat data for HTML elements. https://developer.mozilla.org/docs/Web/HTML labels Sep 25, 2024
Copy link
Contributor

@fred-wang fred-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In term of APIs, we also need to:

@github-actions github-actions bot added the data:http 🚠 Compat data for HTTP features. https://developer.mozilla.org/docs/Web/HTTP label Oct 1, 2024
@@ -40,6 +40,39 @@
"deprecated": false
}
},
"fetchpriority": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fred-wang This adds fetchpriority subfeature to 103 status. The values mirror those for the <link> feature - though for chrome this header wasn't implemented until 103, so I used that instead of 101 (which was the value used in link). Reasonable?

My understanding is that this header may be sent by a server prior to returning the final resource to indicate resources that the browser is likely going to want to request early. So basically this might be used to tell the browser to start loading a resource at higher priority before it parses the resource and finds the <link> tag that suggests it should do so - right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether/when chrome or webkit implemented the 103 header, but yeah that seems reasonable.

I think your understanding is correct, but actually this is also true for e.g. 200 OK. The subtility here is that Early Hints can be sent before the final response has arrived, allowing resources to be loaded even earlier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chrome implemented 103 preloads (Safari only supports preconnects in 103 now, where fetchpriority is not relevant so can probably be ignored for now).

It appears from the WPT's that you added that Chrome doens't error on fetchpriority on 103s:
https://wpt.fyi/results/loading/early-hints/preload-fetchpriority.h2.window.html?label=experimental&label=master&aligned
But we don't think it does anything special with it.

To us in Chrome it doesn't really make sense here as 1) high priority requests will be requested right anyway as soon as these are seen in headers and 2) low priority requests really shouldn't be sent in 103 requests. Not sure if Firefox was thinking of specific use cases for this that I'm not seeing? Or if it was just easier to support it everywhere rather than carve out an exception for link headers?

So you can argue it's "supported" in Chrome in that in that it won't error, but not sure if it's more confusing than useful to add in here?

CC: @pmeenan

Copy link
Collaborator

@hamishwillee hamishwillee Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fred-wang Can you comment on the point immediately above ^^^: Not sure if Firefox was thinking of specific use cases for this that I'm not seeing? Or if it was just easier to support it everywhere rather than carve out an exception for link headers?.

@tunetheweb My assumption here was that if a browser sees a Link header it would download it, but the priority would still be affected by settings like preload and fetchpriority. In otherwords, if you have a lot of stuff to download, the additional prioritisation hint is still useful. I'm pretty ignorant of this stuff, so hopefully @fred-wang can clarify what he things support means for a browser.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinks supporting fetchpriority in 103 came after a review by @mb and I believed it turned out that implementing the generic support for Link header implied support for 103 too. In https://bugzilla.mozilla.org/show_bug.cgi?id=1882084 in addition to the basic test mentioned we also check that fetchpriority affects our internal priority in 103 header. But I don't think we had any specific use case in mind besides supporting what's the spec says. So I agree, probably documenting support for the generic Link header is the right thing to do...

}
}
},
"fetchpriority": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mirrors the <link> element for these features, which were added after introduction of LINK header. Ditto other subfeatures above. Assumption is anything that was implemented in link element is also in the header as per the docs. This is definitely true for fetchpriority on FF.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no support difference compared to <link>, I would suggest not to document these features here.

cc @Elchi3

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are potentially differences- for a start, the Link header appears much more recently than the tag (thought that is captured), and more generally, there are things supported on the tag that are not supported via this header.

IMO it matches "policy" to document these as I have done.

Josh points out in discussion the bcd channel that just documenting the ones that come in different versions is confusing for users - i.e. his take is that once you have added one subfeature you should add all of them. I agree, but that also is against the policy.

Upshot, I don't plan to change this unless @Elchi3 or @queengooborg can confirm and update policy to reflect that.

@hamishwillee
Copy link
Collaborator

@fred-wang I've added the extra information. Does this look sane to you?

@Elchi3 Can I get this in before you do your monthly FF update please.

Copy link
Contributor

@fred-wang fred-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hamishwillee Thanks. yes, this looks good to me.

Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Ignore this, I wasn't aware of Removal of irrelevant flag data.)

Instead of replacing the existing support statement, let's make this an array:

"firefox": [
  {
    // Previous statement
    "version_removed": "132",
  },
  {
    // New statement
  }
],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API data:html 📄 Compat data for HTML elements. https://developer.mozilla.org/docs/Web/HTML data:http 🚠 Compat data for HTTP features. https://developer.mozilla.org/docs/Web/HTTP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants